Skip to content

Add manage search query rules permission#97941

Merged
kderusso merged 6 commits intoelastic:mainfrom
kderusso:kderusso/add-manage-query-rules-privilege
Jul 27, 2023
Merged

Add manage search query rules permission#97941
kderusso merged 6 commits intoelastic:mainfrom
kderusso:kderusso/add-manage-query-rules-privilege

Conversation

@kderusso
Copy link
Copy Markdown
Member

Adds a permission to manage search query rules.

@kderusso kderusso added >non-issue :Security/Authorization Roles, Privileges, DLS/FLS, RBAC/ABAC :EnterpriseSearch/Application Enterprise Search Team:Enterprise Search Meta label for Enterprise Search team labels Jul 25, 2023
@github-actions
Copy link
Copy Markdown
Contributor

Documentation preview:

@elasticsearchmachine elasticsearchmachine added Team:Security Meta label for security team v8.10.0 labels Jul 25, 2023
@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-security (Team:Security)

@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/ent-search-eng (Team:Enterprise Search)

@kderusso
Copy link
Copy Markdown
Member Author

Relates to PR to manage synonyms: #97853

@kderusso kderusso force-pushed the kderusso/add-manage-query-rules-privilege branch from 602accf to b6f38ee Compare July 25, 2023 18:58
@kderusso kderusso requested a review from a team July 26, 2023 12:23
@kderusso kderusso force-pushed the kderusso/add-manage-query-rules-privilege branch from a052b63 to c8e0204 Compare July 26, 2023 13:48

public static final DeleteQueryRulesetAction INSTANCE = new DeleteQueryRulesetAction();
public static final String NAME = "cluster:admin/xpack/query_rules/delete";
public static final String NAME = "cluster:admin/search/query_rules/delete";
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why did we change this?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I renamed this based on the discussion we had to make a manage search privilege that these fall under. If we don't want to do this I can change it back to xpack. WDYT?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As mentioned in #97853, I don't think that's necessary. I also don't see a common grouping like that in the rest of the actions. I'd vote for keeping as is unless we have a good reason not to.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I'll change them back. My bad, I misunderstood the goal.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On second thought, should it be xpack/search/query_rules?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd still say that we don't need the /search/ prefix. I find them more descriptive without it. Not a strong opinion though.

Copy link
Copy Markdown
Member

@ioanatia ioanatia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we have some yaml tests similar to what we had for #97853 ?

@carlosdelest
Copy link
Copy Markdown
Member

should we have some yaml tests similar to what we had for #97853 ?

++. We could probably achieve this by adding auth headers to all the existing tests, and adding an additional use case with a user with no permissions.

Copy link
Copy Markdown
Member

@carlosdelest carlosdelest left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👏

@kderusso kderusso merged commit 1e61728 into elastic:main Jul 27, 2023
@kderusso kderusso deleted the kderusso/add-manage-query-rules-privilege branch July 8, 2024 17:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:EnterpriseSearch/Application Enterprise Search >non-issue :Security/Authorization Roles, Privileges, DLS/FLS, RBAC/ABAC Team:Enterprise Search Meta label for Enterprise Search team Team:Security Meta label for security team v8.10.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants